Mark Kettenis <mark.kette...@xs4all.nl> writes:

>> From: Dave Voutila <d...@sisu.io>
>> Date: Tue, 09 Jan 2024 09:19:56 -0500
>>
>> Stefan Fritsch <s...@openbsd.org> writes:
>>
>> > On 08.01.24 22:24, Alexander Bluhm wrote:
>> >> Hi,
>> >> When running a guest in vmm and doing ifconfig operations on vio
>> >> interface, I can crash the guest.
>> >> I run these loops in the guest:
>> >> while doas ifconfig vio1 inet 10.188.234.74/24; do :; done
>> >> while doas ifconfig vio1 -inet; do :; done
>> >> while doas ifconfig vio1 down; do :; done
>> >> And from host I ping the guest:
>> >> ping -f 10.188.234.74
>> >
>> > I suspect there is a race condition in vmd. The vio(4) kernel driver
>> > resets the device and then frees all the mbufs from the tx and rx
>> > rings. If vmd continues doing dma for a bit after the reset, this
>> > could result in corruption. From this code in vmd's vionet.c
>> >
>> >                 case VIODEV_MSG_IO_WRITE:
>> >                         /* Write IO: no reply needed */
>> >                         if (handle_io_write(&msg, dev) == 1)
>> >                                 virtio_assert_pic_irq(dev, 0);
>> >                         break;
>> >
>> > it looks like the main vmd process will just send a pio write message
>> > to the vionet process but does not wait for the vionet process to
>> > actually execute the device reset. The pio write instruction in the
>> > vcpu must complete after the device reset is complete.
>>
>> Are you saying we need to wait for the emulation of the OUT instruction
>> that the vcpu is executing? I don't believe we should be blocking the
>> vcpu here as that's not how port io works with real hardware. It makes
>> no sense to block on an OUT until the device finishes emulation.
>
> Well, I/O address space is highly synchronous.  See 16.6 "Ordering
> I/O" in the Intel SDM.  There it clearly states that execution of the
> next instruction after an OUT instruction is delayed intil the store
> completes.  Now that isn't necessarily the same as completing all
> device emulation for the device.  But it does mean the store has to
> reach the device register before the next instruction gets executed.
>

Interesting. I think in this case since even if the very next
instruction is an IN to read from the same register, it's being
serialized in the virtio device process in vmd. While the vcpu may
continue forward immediately after the OUT event is relayed to the
device, an IN *does* block in the current multi-process design and
waits for the response of the register value from the device process.

Since the virtio network device is single threaded (currently), ordering
should be preserved and we should always be capable of providing the
value written via OUT as a response to the IN. Assuming no external
event in the device mutates the register value in between.

I'm not ruling out a bug in the device reset code by any means, but I'm
not convinced that vmd is violating any guarantees of the Intel
architecture with the current design.

> Yes, this is slow.  Avoid I/O address space if you can; use
> Memory-Mapped I/O instead.

Well in hypervisor-land that replaces one problem with another :)

-dv

Reply via email to