virtio: Cancel and join threads when exiting devices devices

I'm experiencing a segmentation fault in lkvm where it may crash after
powering off a guest machine that uses a virtio network device.
Th

virtio: Cancel and join threads when exiting devices devices

I'm experiencing a segmentation fault in lkvm where it may crash after
powering off a guest machine that uses a virtio network device.
The crash is hard to reproduce, because looks like it only happens
when the guest machine is powering off while extra virtio threads is
doing some work,
when it happens lkvm crashes in the function virtio_net_rx_thread
while attempting to read invalid guest physical memory,
because guest physical memory was unmapped.

I've isolated the problem and looks like when lkvm exits it unmaps the
guest memory while virtio device extra threads may still be executing.
I noticed most virtio devices are not executing pthread_cancel +
pthread_join to synchronize extra threads when exiting,
to make sure this happens I added explicit calls to the virtio device
exit function to all virtio devices,
which should cancel and join all threads before unmapping guest
physical memory, fixing the crash for me.

Signed-off-by: Eduardo Bart <edub4rt@gmail.com>
Link: https://lore.kernel.org/r/20231117170455.80578-2-edub4rt@gmail.com
[will: Added commit message from https://lore.kernel.org/all/CABqCASLWAZ5aq27GuQftWsXSf7yLFCKwrJxWMUF-fiV7Bc4LUA@mail.gmail.com/]
Signed-off-by: Will Deacon <will@kernel.org>

show more ...

Revert "virtio-net: Don't print the compat warning for the default device"

This reverts commit 15757e8e6441d83757c39046a6cdd3e4d74200ce.

Turns out there's a way to disable the default virtio-net de

Revert "virtio-net: Don't print the compat warning for the default device"

This reverts commit 15757e8e6441d83757c39046a6cdd3e4d74200ce.

Turns out there's a way to disable the default virtio-net device: pass
--network mode=none when running a VM.

Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
Link: https://lore.kernel.org/r/20230907171655.6996-2-alexandru.elisei@arm.com
Signed-off-by: Will Deacon <will@kernel.org>

show more ...

virtio-net: Don't print the compat warning for the default device

Compat messages are there to print a warning when the user creates a virtio
device for the VM, but the guest doesn't initialize it.

virtio-net: Don't print the compat warning for the default device

Compat messages are there to print a warning when the user creates a virtio
device for the VM, but the guest doesn't initialize it.

This generally works great, except that kvmtool will always create a
virtio-net device, even if the user hasn't specified one, which means that
each time kvmtool loads a guest that doesn't probe the network interface,
the user will get the compat warning. This can get particularly annoying
when running kvm-unit-tests, which doesn't need to use a network interface,
and the virtio-net warning is displayed after each test.

Let's fix this by skipping the compat message in the case of the
automatically created virtio-net device. This lets kvmtool keep the compat
warnings as they are, but removes the false positive.

Even if the user is relying on kvmtool creating the default virtio-net
device, a missing network interface in the guest is very easy to
discover.

Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
Link: https://lore.kernel.org/r/20230714152909.31723-1-alexandru.elisei@arm.com
Signed-off-by: Will Deacon <will@kernel.org>

show more ...

virtio/vhost: Clear VIRTIO_F_ACCESS_PLATFORM

Vhost interprets the VIRTIO_F_ACCESS_PLATFORM flag as if accesses need
to use vhost-iotlb, and since kvmtool does not implement vhost-iotlb,
vhost will f

virtio/vhost: Clear VIRTIO_F_ACCESS_PLATFORM

Vhost interprets the VIRTIO_F_ACCESS_PLATFORM flag as if accesses need
to use vhost-iotlb, and since kvmtool does not implement vhost-iotlb,
vhost will fail to access the virtqueue.

This fix is preventive. Kvmtool does not set VIRTIO_F_ACCESS_PLATFORM at
the moment but the Arm CCA and pKVM changes will likely hit the issue
(as experienced with the CCA development tree), so we might as well fix
it now.

Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
Link: https://lore.kernel.org/r/20230606130426.978945-18-jean-philippe@linaro.org
Signed-off-by: Will Deacon <will@kernel.org>

show more ...

virtio/vhost: Support line interrupt signaling

To signal a virtqueue, a kernel vhost worker writes an eventfd
registered by kvmtool with VHOST_SET_VRING_CALL. When MSIs are
supported, this eventfd i

virtio/vhost: Support line interrupt signaling

To signal a virtqueue, a kernel vhost worker writes an eventfd
registered by kvmtool with VHOST_SET_VRING_CALL. When MSIs are
supported, this eventfd is connected directly to KVM IRQFD to inject the
interrupt into the guest. However direct injection does not work when
MSIs are not supported. The virtio-mmio transport does not support MSIs
at all, and even with virtio-pci, the guest may use INTx if the irqchip
does not support MSIs (e.g. irqchip=gicv3 on arm64).

In this case, injecting the interrupt requires writing an ISR register
in virtio to signal that it is a virtqueue notification rather than a
config change. Add a thread that polls the vhost eventfd for interrupts,
and notifies the guest. When the guest configures MSIs, disable polling
on the eventfd and enable direct injection.

Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
Link: https://lore.kernel.org/r/20230606130426.978945-17-jean-philippe@linaro.org
Signed-off-by: Will Deacon <will@kernel.org>

show more ...

virtio/net: Warn about enabling multiqueue with vhost

vhost-net requires to open one file descriptor for each TX/RX queue
pair. At the moment kvmtool does not support multi-queue vhost: it
issues al

virtio/net: Warn about enabling multiqueue with vhost

vhost-net requires to open one file descriptor for each TX/RX queue
pair. At the moment kvmtool does not support multi-queue vhost: it
issues all vhost ioctls on the first pair, and the other pairs are
broken. Refuse the enable vhost when the user asks for multi-queue.

Using multi-queue vhost-net also requires creating the tap interface
with the 'multi_queue' parameter.

Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
Link: https://lore.kernel.org/r/20230606130426.978945-15-jean-philippe@linaro.org
Signed-off-by: Will Deacon <will@kernel.org>

show more ...

virtio/net: Fix feature selection

Move VHOST_GET_FEATURES to get_host_features() so the guest is aware of
what will actually be supported. This removes the invalid guess about
VIRTIO_NET_F_MRG_RXBUF

virtio/net: Fix feature selection

Move VHOST_GET_FEATURES to get_host_features() so the guest is aware of
what will actually be supported. This removes the invalid guess about
VIRTIO_NET_F_MRG_RXBUF (if vhost didn't support it, we shouldn't let the
guest negotiate it).

Note the masking of VHOST_NET_F_VIRTIO_NET_HDR when handing features to
vhost. Unfortunately the vhost-net driver interprets VIRTIO_F_ANY_LAYOUT
as VHOST_NET_F_VIRTIO_NET_HDR, which is specific to vhost and forces
vhost-net to supply the vnet header. Since this is done by tap, we don't
want to set the bit.

Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
Link: https://lore.kernel.org/r/20230606130426.978945-12-jean-philippe@linaro.org
Signed-off-by: Will Deacon <will@kernel.org>

show more ...

virtio/vhost: Factor notify_vq_gsi()

All vhost devices should perform the same operations when initializing
the IRQFD. Move it to virtio/vhost.c

This fixes vsock, which didn't go through the irq__a

virtio/vhost: Factor notify_vq_gsi()

All vhost devices should perform the same operations when initializing
the IRQFD. Move it to virtio/vhost.c

This fixes vsock, which didn't go through the irq__add_irqfd() helper
and couldn't be used on systems that require GSI translation (GICv2m).

Also correct notify_vq_gsi() in net.c, to check which virtqueue is being
configured. Since vhost only manages the data queues, we shouldn't try
to setup GSI routing for the control queue. This hasn't been a problem
so far because the Linux guest doesn't use IRQs for the control queue.

Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
Link: https://lore.kernel.org/r/20230606130426.978945-5-jean-philippe@linaro.org
Signed-off-by: Will Deacon <will@kernel.org>

show more ...

virtio/vhost: Factor notify_vq_eventfd()

All vhost devices perform the same operation when setting up the
ioeventfd. Move it to virtio/vhost.c

Reviewed-by: Andre Przywara <andre.przywara@arm.com>
S

virtio/vhost: Factor notify_vq_eventfd()

All vhost devices perform the same operation when setting up the
ioeventfd. Move it to virtio/vhost.c

Reviewed-by: Andre Przywara <andre.przywara@arm.com>
Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
Link: https://lore.kernel.org/r/20230606130426.978945-4-jean-philippe@linaro.org
Signed-off-by: Will Deacon <will@kernel.org>

show more ...

virtio/vhost: Factor vring operation

The VHOST_VRING* ioctls are common to all device types, move them to
virtio/vhost.c

Reviewed-by: Andre Przywara <andre.przywara@arm.com>
Signed-off-by: Jean-Phi

virtio/vhost: Factor vring operation

The VHOST_VRING* ioctls are common to all device types, move them to
virtio/vhost.c

Reviewed-by: Andre Przywara <andre.przywara@arm.com>
Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
Link: https://lore.kernel.org/r/20230606130426.978945-3-jean-philippe@linaro.org
Signed-off-by: Will Deacon <will@kernel.org>

show more ...

virtio: Factor vhost initialization

Move vhost owner and memory table setup to virtio/vhost.c.

This also fixes vsock and SCSI which did not support multiple memory
regions until now (vsock didn't a

virtio: Factor vhost initialization

Move vhost owner and memory table setup to virtio/vhost.c.

This also fixes vsock and SCSI which did not support multiple memory
regions until now (vsock didn't allocate the right region size and would
trigger a buffer overflow).

Reviewed-by: Andre Przywara <andre.przywara@arm.com>
Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
Link: https://lore.kernel.org/r/20230606130426.978945-2-jean-philippe@linaro.org
Signed-off-by: Will Deacon <will@kernel.org>

show more ...

virtio: sanitise virtio endian wrappers

In virtio/scsi.c we had a small hack to avoid compiler warnings when not
using cross-endian support: we were assigning a variable to itself.
This upsets clang

virtio: sanitise virtio endian wrappers

In virtio/scsi.c we had a small hack to avoid compiler warnings when not
using cross-endian support: we were assigning a variable to itself.
This upsets clang:
virtio/scsi.c:63:7: error: explicitly assigning value of variable of type
'struct virtio_device *' to itself [-Werror,-Wself-assign]
This hack was needed because we use *macros* to do the endianess
conversion, and for architectures like x86 the "dev" argument was removed
from the code.

Provide the endianess conversion functions as inline functions, which do
not suffer from the unused problem.
This requires to isolate the "endian" parameter, because there were
*two* different structures used as the first argument(virtio_device and
virt_queue), *both* with an identically defined "endian" member.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
Link: https://lore.kernel.org/r/20230525144827.679651-3-andre.przywara@arm.com
Signed-off-by: Will Deacon <will@kernel.org>

show more ...

Add virtio-transport option and deprecate force-pci and virtio-legacy.

This is a follow-up patch for [0] which proposed the --force-pci option
for riscv. As per the discussion it was concluded to ad

Add virtio-transport option and deprecate force-pci and virtio-legacy.

This is a follow-up patch for [0] which proposed the --force-pci option
for riscv. As per the discussion it was concluded to add virtio-tranport
option taking in four options (pci, pci-legacy, mmio, mmio-legacy).

With this change force-pci and virtio-legacy are both deprecated and
arm's default transport changes from MMIO to PCI as agreed in [0].
This is also true for riscv.

Nothing changes for other architectures.

[0]: https://lore.kernel.org/all/20230118172007.408667-1-rkanwal@rivosinc.com/

Signed-off-by: Rajnesh Kanwal <rkanwal@rivosinc.com>
Link: https://lore.kernel.org/r/20230320143344.404307-1-rkanwal@rivosinc.com
Signed-off-by: Will Deacon <will@kernel.org>

show more ...

net: Use vfork() instead of fork() for script execution

When a script is specified for a guest nic setup, we fork() and execl()s
the script when it is time to execute the script. However this is not

net: Use vfork() instead of fork() for script execution

When a script is specified for a guest nic setup, we fork() and execl()s
the script when it is time to execute the script. However this is not
optimal, given we are running a VM. The fork() will trigger marking the
entire page-table of the current process as CoW, which will trigger
unmapping the entire stage2 page tables from the guest. Anyway, the
child process will exec the script as soon as we fork(), making all
these mm operations moot. Also, this operation could be problematic
for confidential compute VMs, where it may be expensive (and sometimes
destructive) to make changes to the stage2 page tables.

So, instead we could use vfork() and avoid the CoW and unmap of the stage2.

Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
Reviewed-by: Alexandru Elisei <alexandru.elisei@arm.com>
Link: https://lore.kernel.org/r/20220809124816.2880990-1-suzuki.poulose@arm.com
Signed-off-by: Will Deacon <will@kernel.org>

show more ...

virtio: Prepare for more feature bits

Modern virtio uses more than 32 bits of features. Bump the feature
bitfield size to 64 bits.

virtio_set_guest_features() changes in behavior because it will no

virtio: Prepare for more feature bits

Modern virtio uses more than 32 bits of features. Bump the feature
bitfield size to 64 bits.

virtio_set_guest_features() changes in behavior because it will now be
called multiple times, each time the guest writes to a 32-bit slice of
the features.

Signed-off-by: Jean-Philippe Brucker <jean-philippe.brucker@arm.com>
Link: https://lore.kernel.org/r/20220701142434.75170-7-jean-philippe.brucker@arm.com
Signed-off-by: Will Deacon <will@kernel.org>

show more ...

virtio/net: Set vhost backend after queue address

We currently call VHOST_SET_BACKEND from notify_vq_gsi(), which can't
work with modern virtio because vhost checks that the virtqueue is
accessible

virtio/net: Set vhost backend after queue address

We currently call VHOST_SET_BACKEND from notify_vq_gsi(), which can't
work with modern virtio because vhost checks that the virtqueue is
accessible when handling VHOST_SET_BACKEND, and the modern driver
initializes the MSIs before setting up the virtqueue. Move
VHOST_SET_BACKEND to init_vq().

Signed-off-by: Jean-Philippe Brucker <jean-philippe.brucker@arm.com>
Link: https://lore.kernel.org/r/20220701142434.75170-6-jean-philippe.brucker@arm.com
Signed-off-by: Will Deacon <will@kernel.org>

show more ...

virtio/net: Implement VIRTIO_F_ANY_LAYOUT feature

Modern virtio demands that devices do not make assumptions about the
buffer layouts. Currently the user network backend assumes that TX
packets are

virtio/net: Implement VIRTIO_F_ANY_LAYOUT feature

Modern virtio demands that devices do not make assumptions about the
buffer layouts. Currently the user network backend assumes that TX
packets are neatly split between virtio-net header and ethernet frame.
Modern virtio-net usually puts everything into one descriptor, but could
also split the buffer arbitrarily. Handle arbitrary buffer layouts and
advertise the VIRTIO_F_ANY_LAYOUT feature.

Signed-off-by: Jean-Philippe Brucker <jean-philippe.brucker@arm.com>
Link: https://lore.kernel.org/r/20220607170239.120084-14-jean-philippe.brucker@arm.com
Signed-off-by: Will Deacon <will@kernel.org>

show more ...

virtio/net: Prepare for modern virtio

The virtio_net header contains a 'num_buffers' field, used when the
VIRTIO_NET_F_MRG_RXBUF feature is negotiated. The legacy driver does not
present this field

virtio/net: Prepare for modern virtio

The virtio_net header contains a 'num_buffers' field, used when the
VIRTIO_NET_F_MRG_RXBUF feature is negotiated. The legacy driver does not
present this field when the feature is not negotiated. In that case the
header is 2 bytes smaller.

When using the modern virtio transport, the header always contains the
field and in addition the device MUST set it to 1 when the
VIRTIO_NET_F_MRG_RXBUF is not negotiated. Prepare for modern virtio
support by enabling this case once the 'legacy' flag is switched off.

Signed-off-by: Jean-Philippe Brucker <jean-philippe.brucker@arm.com>
Link: https://lore.kernel.org/r/20220607170239.120084-13-jean-philippe.brucker@arm.com
Signed-off-by: Will Deacon <will@kernel.org>

show more ...

virtio/net: Offload vnet header endianness conversion to tap

The conversion of vnet header fields will be more difficult when
supporting the virtio ANY_LAYOUT feature. Since the uip backend doesn't

virtio/net: Offload vnet header endianness conversion to tap

The conversion of vnet header fields will be more difficult when
supporting the virtio ANY_LAYOUT feature. Since the uip backend doesn't
use the vnet header, and since tap can handle that conversion itself,
offload it to tap.

Signed-off-by: Jean-Philippe Brucker <jean-philippe.brucker@arm.com>
Link: https://lore.kernel.org/r/20220607170239.120084-12-jean-philippe.brucker@arm.com
Signed-off-by: Will Deacon <will@kernel.org>

show more ...

virtio: Remove set_guest_features() device op

Now that devices have a status callback, they don't use
set_guest_features() anymore. The negotiated feature set is available in
struct virtio_device. R

virtio: Remove set_guest_features() device op

Now that devices have a status callback, they don't use
set_guest_features() anymore. The negotiated feature set is available in
struct virtio_device. Remove the callback from all devices.

Signed-off-by: Jean-Philippe Brucker <jean-philippe.brucker@arm.com>
Link: https://lore.kernel.org/r/20220607170239.120084-10-jean-philippe.brucker@arm.com
Signed-off-by: Will Deacon <will@kernel.org>

show more ...

virtio: Fix device-specific config endianness

Some legacy virtio drivers expect to read the device-specific config in
guest endianness (2.5.3 "Legacy Interface: A Note on Device
Configuration Space

virtio: Fix device-specific config endianness

Some legacy virtio drivers expect to read the device-specific config in
guest endianness (2.5.3 "Legacy Interface: A Note on Device
Configuration Space endian-ness").

Kvmtool doesn't know the guest endianness until it can probe a VCPU. So
the config fields start in host endianness, and are swapped once the
guest is running. Currently this is done in set_guest_features(), but
that is too late because the driver is allowed to read config fields
before setting feature bits (2.5.2 "Device Requirements: Device
Configuration Space"). In addition some devices don't swap the fields,
and those that do swap the fields do it every time the guest writes the
feature register, which can't work if a device gets reset more than
once.

Initialize the config on device reset. Do it on every reset because in
theory multiple guests could run with different endianness during the
lifetime of the device.

Notes:

* the balloon device uses little-endian (5.5.4.0.0.1 "Legacy Interface:
Device configuration layout").

* the vsock device was introduced after virtio 0.9.5, hence doesn't
describe a legacy interface, but the Linux driver allows to use the
legacy transport, and always reads the 64-bit guest_cid field as
little-endian.

* the specification does not describe the 9p device, but the Linux
driver uses guest-endian helpers.

* the specification does not explicitly forbid a driver from reading the
configuration at any time, but a driver must follow the sequence from
3.1.1 "Driver Requirements: Device Initialization", where the driver
is allowed to read the config after setting the DRIVER status bit. It
should therefore be safe to keep dealing with guest endianness only on
device reset, and not on the first config access.

Signed-off-by: Jean-Philippe Brucker <jean-philippe.brucker@arm.com>
Link: https://lore.kernel.org/r/20220607170239.120084-8-jean-philippe.brucker@arm.com
Signed-off-by: Will Deacon <will@kernel.org>

show more ...

virtio: Support modern virtqueue addresses

Modern virtio devices can use separate buffer for descriptors, available
and used rings. They can also use 64-bit addresses instead of 44-bit.
Rework the v

virtio: Support modern virtqueue addresses

Modern virtio devices can use separate buffer for descriptors, available
and used rings. They can also use 64-bit addresses instead of 44-bit.
Rework the virtqueue initialization function to support modern virtio.

Signed-off-by: Jean-Philippe Brucker <jean-philippe.brucker@arm.com>
Link: https://lore.kernel.org/r/20220607170239.120084-6-jean-philippe.brucker@arm.com
Signed-off-by: Will Deacon <will@kernel.org>

show more ...

virtio: Factor virtqueue initialization

All virtio devices perform the same set of operations when initializing
their virtqueues. Move it to virtio core.

Signed-off-by: Jean-Philippe Brucker <jean-

virtio: Factor virtqueue initialization

All virtio devices perform the same set of operations when initializing
their virtqueues. Move it to virtio core.

Signed-off-by: Jean-Philippe Brucker <jean-philippe.brucker@arm.com>
Link: https://lore.kernel.org/r/20220607170239.120084-5-jean-philippe.brucker@arm.com
Signed-off-by: Will Deacon <will@kernel.org>

show more ...

virtio: Check for overflows in QUEUE_NOTIFY and QUEUE_SEL

This patch checks for overflows in QUEUE_NOTIFY and QUEUE_SEL in
the PCI and MMIO operation handling paths. Further, the return
value type o

virtio: Check for overflows in QUEUE_NOTIFY and QUEUE_SEL

This patch checks for overflows in QUEUE_NOTIFY and QUEUE_SEL in
the PCI and MMIO operation handling paths. Further, the return
value type of get_vq_count is changed from int to uint since negative
doesn't carry any semantic meaning.

Reviewed-by: Alexandru Elisei <alexandru.elisei@arm.com>
Signed-off-by: Martin Radev <martin.b.radev@gmail.com>
Link: https://lore.kernel.org/r/20220509203940.754644-6-martin.b.radev@gmail.com
Signed-off-by: Will Deacon <will@kernel.org>

show more ...

virtio: Sanitize config accesses

The handling of VIRTIO_PCI_O_CONFIG is prone to buffer access overflows.
This patch sanitizes this operation by using the newly added virtio op
get_config_size. Any

virtio: Sanitize config accesses

The handling of VIRTIO_PCI_O_CONFIG is prone to buffer access overflows.
This patch sanitizes this operation by using the newly added virtio op
get_config_size. Any access which goes beyond the config structure's
size is prevented and a failure is returned.

Additionally, PCI accesses which span more than a single byte are prevented
and a warning is printed because the implementation does not currently
support the behavior correctly.

Reviewed-by: Alexandru Elisei <alexandru.elisei@arm.com>
Signed-off-by: Martin Radev <martin.b.radev@gmail.com>
Link: https://lore.kernel.org/r/20220509203940.754644-5-martin.b.radev@gmail.com
Signed-off-by: Will Deacon <will@kernel.org>

show more ...