Page MenuHomeFreeBSD

add virtio single-machine testing framework
Changes PlannedPublic

Authored by emil_etsalapatis.com on May 26 2024, 4:24 PM.
Tags
None
Referenced Files
Unknown Object (File)
Tue, Jul 16, 12:48 PM
Unknown Object (File)
Sat, Jul 13, 10:07 PM
Unknown Object (File)
Thu, Jul 11, 6:34 AM
Unknown Object (File)
Wed, Jul 10, 5:35 AM
Unknown Object (File)
Sat, Jul 6, 8:55 PM
Unknown Object (File)
Sat, Jul 6, 8:37 PM
Unknown Object (File)
Mon, Jul 1, 10:26 AM
Unknown Object (File)
Thu, Jun 27, 8:55 PM

Details

Reviewers
asomers
Summary

Testing virtio drivers like virtiofs is currently not possible without host/guest
cooperation, which is impractical in a CI environment. Introduce a virtio debug transport
that allows device emulation in the same machine as the driver.

Test Plan

WIP

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 57923
Build 54811: arc lint + arc unit

Event Timeline

asomers added a subscriber: asomers.

I think you need to add a definition for the virtio_dbg module in sys/modules. Oh, and how do you use it? Eventually we want ATF tests, of course. But I assume for now you've been manually using it somehow?

tests/sys/virtio/Makefile
1

You need to add these definitions. PACKAGE determines which file set in the DVD the file will belong to. The other too ensure that the file will be installed into the correct location.

This revision now requires changes to proceed.May 27 2024, 11:45 PM

I think you need to add a definition for the virtio_dbg module in sys/modules. Oh, and how do you use it? Eventually we want ATF tests, of course. But I assume for now you've been manually using it somehow?

Will do, I have been building it as part of the kernel so that's probably why it didn't need a definition in sys/modules. I have been running this manually.

I've been thinking on how to get this code to be more reviewable. Should I maybe try to get it published as a port instead of submitting it as a patch to src/? Most of this code is self-contained, and its size means it will be next to impossible to review in its current state. If we go down the port route, only changes we would be applying to src/ would be non-functional - most would be about turning some static functions global.

Alternatively, I can break this diff apart, get the virtio-dbg transport reviewed since it is more reasonably sized, and either spin off the userspace device emulation component as a port or introduce it later. Which do you think is more reasonable?

Will do, I have been building it as part of the kernel so that's probably why it didn't need a definition in sys/modules. I have been running this manually.

I've been thinking on how to get this code to be more reviewable. Should I maybe try to get it published as a port instead of submitting it as a patch to src/? Most of this code is self-contained, and its size means it will be next to impossible to review in its current state. If we go down the port route, only changes we would be applying to src/ would be non-functional - most would be about turning some static functions global.

Alternatively, I can break this diff apart, get the virtio-dbg transport reviewed since it is more reasonably sized, and either spin off the userspace device emulation component as a port or introduce it later. Which do you think is more reasonable?

Splitting it apart sounds good. Especially if there's some way to exercise the virtio-dbg transport without the userspace device emulation. Then splitting it apart would be a very good idea.

Will do, I have been building it as part of the kernel so that's probably why it didn't need a definition in sys/modules. I have been running this manually.

I've been thinking on how to get this code to be more reviewable. Should I maybe try to get it published as a port instead of submitting it as a patch to src/? Most of this code is self-contained, and its size means it will be next to impossible to review in its current state. If we go down the port route, only changes we would be applying to src/ would be non-functional - most would be about turning some static functions global.

Alternatively, I can break this diff apart, get the virtio-dbg transport reviewed since it is more reasonably sized, and either spin off the userspace device emulation component as a port or introduce it later. Which do you think is more reasonable?

Splitting it apart sounds good. Especially if there's some way to exercise the virtio-dbg transport without the userspace device emulation. Then splitting it apart would be a very good idea.

Sounds good, I'll break apart the transport code into separate diffs. We can test the transport w/o the emulation up to a point, e.g., the basic open/mmap/close code for the control device, so we can start with just virtio-dbg and go from there.

kib added inline comments.
sys/dev/virtio/dbg/virtio_dbg.c
447

Why do you use pmap_enter() there, and not pmap_qenter()? Is this because the code was copied from kmem_alloc_contig()?

Might be, generalize kmem_alloc_contig() to allow it to take a caller-supplied object, instead of always assuming kernel_object?

523

I suspect at modern times this line should be spelled as bus_topo_lock().

557

Why NOWAIT?

569

Indent for continuation line should be +4 spaces.

664

Oh, so it is amd64 only (- ARM64) ?

791

Why is this NOWAIT?

802

I.e. the code is limited to amd64/arm64? Why?

Look at the sfbuf(9) facility, or might be uimove_fromphys(9).